-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resolve 413 errors by setting Flask's MAX_FORM_MEMORY_SIZE
limit to 20 MB
#823
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #823 +/- ##
==========================================
+ Coverage 99.60% 99.63% +0.02%
==========================================
Files 93 93
Lines 7134 7142 +8
==========================================
+ Hits 7106 7116 +10
+ Misses 28 26 -2 ☔ View full report in Codecov by Sentry. |
Would it be better to practice defense in depth and set some reasonably high limit on body size - maybe 10 or 20 megabytes - instead of disabling the limit entirely? Yes, nginx or similar should be there to place actual limits. But what if it isn't in some particular case, or its limits have been disabled? |
Quality Gate passedIssues Measures |
Now the limit is set to 20 MB. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
MAX_FORM_MEMORY_SIZE
limitMAX_FORM_MEMORY_SIZE
limit to 20 MB
For the record, I also tried to look how the value for MAX_FORM_MEMORY_SIZE could be set by a user via environment variable, but did not find the right way to do it with Annif. The env |
… 20 MB (#823) * Resolve 413 Errors by disabling Flask's MAX_FORM_MEMORY_SIZE limit * Set MAX_FORM_MEMORY_SIZE limit to 20 MB
When evaluating the new, to-be-published YSO projects, the requests whose body exceeded 500 KB resulted to
413 Client Error: Request Entity Too Large for url
error, which did not happen on the previous projects update round on May.The cause was the limit introduced in Werkzeug 3.1.0 (pallets/werkzeug#2965):
In Flask 3.1 there is a new config setting
MAX_FORM_MEMORY_SIZE
to set a value for this limit.This PR uses the setting to
disable the limit altogetherincrease the value from the default (500 KB) to 20 MB.In any case, on production Annif should be run behind e.g. NGINX, which can be used to set request size limits more tightly.